Conversation
CatarinaGamboa
left a comment
There was a problem hiding this comment.
For the nullness check, I think we need more cases, like what if we declare the var and it doesn't have a value?
There are full tools just for the nullness checking (e.g., NullAway https://github.com/uber/NullAway)
PS: this would be so much easier if it were 2 prs, one for null and one for boxed types -- the boxed types would be an easy squash and merge
...va-verifier/src/main/java/liquidjava/processor/refinement_checker/RefinementTypeChecker.java
Outdated
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
CatarinaGamboa
left a comment
There was a problem hiding this comment.
This is nice to have, but I have some questions (we already discussed some offline):
obj != nulland state. It would be great if we didnt have to pollute the refinements with always having to writethis != null. Maybe we can assume that all object states imply!= null?- null of fields are trickier, check the comment below
| Integer x; // implicit null | ||
|
|
||
| void test() { | ||
| mustBeNull(x); |
There was a problem hiding this comment.
But we don't know if this is null right? how do we know? what if there was a constructor, what if another method is called first and sets this fiels? what are the assumptions here?
There was a problem hiding this comment.
Yeah you're right. I'll try to handle those cases.
There was a problem hiding this comment.
Well, I think that is a different problem, not actually related to nulls.
Consider this example, which fails the verification:
public class TestField {
boolean x; // even if we initialize x with true the same error occurs btw
@Refinement("_ == true")
boolean test() {
setValue();
return x; // Refinement Error: #ret_0 == this#x is not a subtype of #ret_0 == true
}
void setValue() {
x = true;
}
}However, in this case, the verification passes:
public class TestField {
boolean x;
@Refinement("_ == true")
boolean test() {
x = true;
return x; // ok
}
}This means that the specific problem you mentioned is about how field updates across method calls are handled. The typechecker does not propagate the effect of setValue(), meaning field mutations are not reflected in the context after a method or constructor is called.
Therefore, I think that problem is unrelated to this PR and we should open its own issue.
What do you think @CatarinaGamboa?
There was a problem hiding this comment.
Okay several points:
1 - We cannot handle the field tracking atm, only with Latte - aliasing tracking extension, that we are working on in parallel. So you don't need to try and tackle this issue here, definitely, I agree.
2 - I think my main issue in the example was with mustBeNull, like we can't know for sure, we maybe should not track the nullness of them when entering methods, we would need an if to verify its value like
void test() {
if ( x == null){
mustBeNull(x);
} else {
mustBeNotNull(x);
}This I think we should support, but we cannot be certain of the value at the start of the method.
Does this make sense?
We would have the code slilghtly polluted by ifs bit at least we check the values.
There was a problem hiding this comment.
1 - Yeah you're right, I didn't even think of Latte.
2 - I agree. I'll change that test.
There was a problem hiding this comment.
for 2 - I think we should point out an issue with the call to mustBeNull(x) without the if
This PR adds support for null checking in LiquidJava.
Changes
Refinements Language
nullliteral to the grammarLiteralNullAST nodeTyping
nullcan be assigned to any non-primitive type_ != null, while the literalnullcarries the refinement_ == null_ == nullZ3 Translation
java.lang.Objectnullliterals are converted to the sort of the other operand to match both sortsSimplification
s == -1 && s != nullsimplifies tos == -1instead of-1 != nullTesting